Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding make_column_transformer #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wlandecker
Copy link

Gave this issue a shot: #13

ColumnTransformer breaks from Pipeline and FeatureUnion's pattern by accepting a list of (encoder, 'column_name') tuples, instead of just a list of encoders. As a result, one question for this PR is whether searchgrid should pass a transformer name to ColumnTransformer.transformer that only contains the encoder class name, or if it should also include the column name. To be consistent with make_pipeline and make_union, I chose only the encoder class name.

@jnothman I'd love any comments or suggestions!

@coveralls
Copy link

coveralls commented Nov 30, 2018

Coverage Status

Coverage increased (+0.1%) to 99.115% when pulling 3d5c30d on wlandecker:add_column_transformer into 2c25bce on jnothman:master.

@wlandecker
Copy link
Author

wlandecker commented Nov 30, 2018

Oh shoot, I just realized that in ColumnTransformer, column_name is expected to be a list of column name strings, not a string itself. Let me fix that first...

Fixed.

@jnothman
Copy link
Owner

jnothman commented Dec 3, 2018 via email

@wlandecker
Copy link
Author

No worries! Take your time.

Copy link
Owner

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@@ -2,6 +2,7 @@
from collections import defaultdict as _defaultdict
import itertools as _itertools

from sklearn.compose import ColumnTransformer as _ColumnTransformer
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this code scikit-learn 0.19-compatible by importing this in make_column_transformer and skipping the corresponding tests in old versions.

* if a step has estimators of mixed type, the step is named 'alt'
* if there are multiple steps of the same name using the above rules,
a suffix '-1', '-2', etc. is added.
"""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a usage example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants